Skip to content

test(desktop): improve market mvvm test coverage with msw#14920

Merged
mcayuelas-ledger merged 1 commit intodevelopfrom
support/market-mvvm-test-coverage
Mar 3, 2026
Merged

test(desktop): improve market mvvm test coverage with msw#14920
mcayuelas-ledger merged 1 commit intodevelopfrom
support/market-mvvm-test-coverage

Conversation

@mcayuelas-ledger
Copy link
Contributor

@mcayuelas-ledger mcayuelas-ledger commented Mar 2, 2026

✅ Checklist

  • npx changeset was attached.
  • Covered by automatic tests.
  • Impact of the changes:
    - no production code changes, test-only additions
    - market feature hooks and components coverage

📝 Description

this pr adds unit tests for the market mvvm layer in ledger live desktop, improving coverage for hooks, view models, and view components.

test files added:

  • useMarketCoin.test.ts — star/unstar, range/counter currency changes, color resolution, data loading with msw handlers for coingecko and dada apis
  • useMarketActions.test.ts — availability flags (buy/swap/stake), action callbacks with msw for dada lazy currency resolution, feature flag deactivation via initialstate
  • useRowItemViewModel.test.ts — hasActions logic, navigation, star click, price change, label pass-through
  • RowItemView.test.tsx — rendering, action buttons visibility, sparkline chart, click handlers
  • ListData.test.tsx — empty/loading/error states, starred filtering, data rendering
  • shared.ts — shared test constants for market and dada api endpoints

approach: replaced jest.mock with msw where applicable for more realistic integration testing of rtk query hooks.

❓ Context


🧐 Checklist for the PR Reviewers

  • The code aligns with the requirements described in the linked JIRA or GitHub issue.
  • The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
  • There are no undocumented trade-offs, technical debt, or maintainability issues.
  • The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
  • Any new dependencies have been justified and documented.
  • Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

@mcayuelas-ledger mcayuelas-ledger marked this pull request as ready for review March 2, 2026 13:17
@mcayuelas-ledger mcayuelas-ledger requested a review from a team as a code owner March 2, 2026 13:17
@live-github-bot live-github-bot bot added the desktop Has changes in LLD label Mar 2, 2026
Copilot AI review requested due to automatic review settings March 2, 2026 13:17
@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch from d9a2906 to 3a5b8b5 Compare March 2, 2026 13:20
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2026

⚠️ E2E tests are required

Changes detected require e2e testing before merge (even before asking for any review).

🖥️ Desktop

-> Run Desktop E2E

  • Select "Run workflow"
  • Branch: support/market-mvvm-test-coverage
  • Device: nanoSP or stax

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Jest tests for the Ledger Live Desktop Market MVVM layer (src/mvvm/features/Market), aiming to improve coverage for hooks, view models, and view components while using MSW for more realistic API behavior.

Changes:

  • Added unit tests for Market MVVM hooks (useMarketCoin, useMarketActions) using MSW handlers.
  • Added unit tests for Market row VM/view and list rendering (useRowItemViewModel, RowItemView, ListData).
  • Added shared test constants for Market/DADA endpoints and a changeset entry.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
apps/ledger-live-desktop/src/mvvm/features/Market/hooks/tests/useMarketCoin.test.ts Adds MSW-backed tests for coin detail hook behaviors (star, range/counter currency, color, loading/chart data).
apps/ledger-live-desktop/src/mvvm/features/Market/hooks/tests/useMarketActions.test.ts Adds tests for buy/swap/stake availability and callbacks, including DADA resolution via MSW.
apps/ledger-live-desktop/src/mvvm/features/Market/hooks/tests/shared.ts Introduces shared constants for MSW endpoints and an empty DADA response fixture.
apps/ledger-live-desktop/src/mvvm/features/Market/components/tests/ListData.test.tsx Adds tests for list rendering using a mocked virtualizer and mocked row VM.
apps/ledger-live-desktop/src/mvvm/features/Market/components/RowItem/tests/useRowItemViewModel.test.ts Adds tests for row VM logic (actions visibility, navigation, star click, price change selection, labels).
apps/ledger-live-desktop/src/mvvm/features/Market/components/RowItem/tests/RowItemView.test.tsx Adds tests for row view rendering and interactions (labels, action buttons, star, sparkline).
.changeset/wise-rivers-change.md Adds a changeset entry for the desktop package.

@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch 2 times, most recently from 6a22b95 to 608d5a7 Compare March 2, 2026 14:17
Copilot AI review requested due to automatic review settings March 2, 2026 14:28
@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch from 608d5a7 to ca262bd Compare March 2, 2026 14:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

expect(result.current.isLoadingCurrency).toBe(false);
});

expect(result.current.color).toBe("#BBB0FF");
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion hard-codes the dark-theme primary.c80 hex value ("#BBB0FF"), which can change when design tokens/palettes evolve, making the test brittle. Prefer asserting against the theme value (e.g., read it via useTheme() in the same renderHook) or asserting that getCurrencyColor is not called and the fallback branch is used without depending on a specific hex.

Suggested change
expect(result.current.color).toBe("#BBB0FF");
expect(getCurrencyColor).not.toHaveBeenCalled();
expect(result.current.color).toBeTruthy();

Copilot uses AI. Check for mistakes.
Comment on lines +76 to +82
it("should return availableOnBuy=false when currency ledgerIds are deactivated", () => {
const deactivatedCurrency = { ...mockCurrency, ledgerIds: ["arbitrum"] };

const { result } = renderMarketActionsHook(deactivatedCurrency);

expect(result.current.availableOnBuy).toBe(false);
});
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test depends on the repository's default feature-flag configuration to have the "arbitrum" currency deactivated. To keep the test deterministic and self-contained, explicitly set settings.overriddenFeatureFlags in the renderHook initialState (or otherwise control useCurrenciesUnderFeatureFlag) so the expected availability doesn't change if defaults are updated.

Copilot uses AI. Check for mistakes.

render(<ListData {...defaultProps} toggleStar={toggleStar} rowVirtualizer={rowVirtualizer} />);

fireEvent.click(screen.getByTestId("market-BTC-star-button"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can't use user.click?

it("should render currency name and ticker", () => {
render(<RowItemView {...createDefaultProps()} />);

expect(screen.getByText("Bitcoin")).toBeInTheDocument();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(screen.getByText("Bitcoin")).toBeInTheDocument();
expect(screen.getByText("Bitcoin")).toBeVisible();

@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch from ca262bd to 4db5be6 Compare March 2, 2026 18:34
Copilot AI review requested due to automatic review settings March 3, 2026 08:24
@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch from 4db5be6 to f63db7e Compare March 3, 2026 08:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

@mcayuelas-ledger mcayuelas-ledger force-pushed the support/market-mvvm-test-coverage branch from f63db7e to 90c8537 Compare March 3, 2026 08:47
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 3, 2026

@mcayuelas-ledger mcayuelas-ledger merged commit b72a5d1 into develop Mar 3, 2026
60 checks passed
@mcayuelas-ledger mcayuelas-ledger deleted the support/market-mvvm-test-coverage branch March 3, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

desktop Has changes in LLD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants